Rearrangement Algorithm Implementation #1473
Conversation
|
@ckcks12 this looks great; we're very excited about getting this merged! |
|
thanks for doing this, hoping to get some time to review in the next few days! |
|
Looks like a lot of people are waiting for this PR to be merge! @jquense, did you get a chance to review this PR? Appreciate your time and help here 🙏 |
|
Nice fix. I also need to have a couple of more enhancement in UI. I hope the community welcomes them as well. |
There was a problem hiding this comment.
Hey there! Had quick, functionally this looks ok, but the structure isn't acceptable at the moment. There seems like there is a lot of redundant work done in getStyledEvents and then deleted or altered in DayColumn. No layout work should be in DayColumn, that's why DayEventLayout exists.
I would add dayLayoutAlgorithm a as prop and provide two options, the current overlapping, and non overlapping implementations. Each implementation should be its own file and essentially just export getStyledEvents which should be the main API area for doing this. That way we can also allow users to provide their own function for dayLayoutAlgorithm if they want customized logic
src/DayColumn.js
Outdated
| import TimeSlotGroup from './TimeSlotGroup' | ||
| import TimeGridEvent from './TimeGridEvent' | ||
|
|
||
| function dfs(node, maxIdx, visited) { |
& move Event class into overlap algorithm
| { | ||
| id: 16, | ||
| title: 'Video Record', | ||
| start: new Date(2015, 3, 14, 15, 30, 0), | ||
| end: new Date(2015, 3, 14, 19, 0, 0), | ||
| }, | ||
| { | ||
| id: 17, | ||
| title: 'Dutch Song Producing', | ||
| start: new Date(2015, 3, 14, 16, 30, 0), | ||
| end: new Date(2015, 3, 14, 20, 0, 0), | ||
| }, | ||
| { | ||
| id: 18, | ||
| title: 'Itaewon Halloween Meeting', | ||
| start: new Date(2015, 3, 14, 16, 30, 0), | ||
| end: new Date(2015, 3, 14, 17, 30, 0), | ||
| }, | ||
| { | ||
| id: 19, | ||
| title: 'Online Coding Test', | ||
| start: new Date(2015, 3, 14, 17, 30, 0), | ||
| end: new Date(2015, 3, 14, 20, 30, 0), | ||
| }, | ||
| { | ||
| id: 20, | ||
| title: 'An overlapped Event', | ||
| start: new Date(2015, 3, 14, 17, 0, 0), | ||
| end: new Date(2015, 3, 14, 18, 30, 0), | ||
| }, | ||
| { | ||
| id: 21, | ||
| title: 'Phone Interview', | ||
| start: new Date(2015, 3, 14, 17, 0, 0), | ||
| end: new Date(2015, 3, 14, 18, 30, 0), | ||
| }, | ||
| { | ||
| id: 22, | ||
| title: 'Cooking Class', | ||
| start: new Date(2015, 3, 14, 17, 30, 0), | ||
| end: new Date(2015, 3, 14, 19, 0, 0), | ||
| }, | ||
| { | ||
| id: 23, | ||
| title: 'Go to the gym', | ||
| start: new Date(2015, 3, 14, 18, 30, 0), | ||
| end: new Date(2015, 3, 14, 20, 0, 0), | ||
| }, |
There was a problem hiding this comment.
These are for overlapping situation
| top: stringifyPercent(top), | ||
| [rtl ? 'right' : 'left']: stringifyPercent(xOffset), | ||
| width: stringifyPercent(width), | ||
| height: stringifyPercent(height), |
There was a problem hiding this comment.
Sometimes the algorithm needs calc() so I just made it as 'string' || 'number'. number will be followed by %.
Maybe you think it is no good that I didn't make it all string only. Frankly I tried it but you know that plain css string is quite annoying for testing, right?
I also tried to parse() and floor() but... yeah... too much. so this is why.
|
@jquense Hallo~ I made it as strategy pattern like pluggable design. 😆
And from the top of the library, the and also I created a demo section for Sooooooo hope you like it 👀 |
|
just need a moment to try it out then gtg |
|
@jquense did you find any blockers? |
|
Very excited for this! Thanks for all the work on this everyone involved. |
|
Having overlap strategy as an option to Calendar would be very helpful. I also need all events visible at the cost of ugly presentation. |
|
@jquense Thanks for merging :) Can we please push this to npm and increment the package version as a Looking forward to this update 😄 |
7 participants
#1464
From
To
Algorithm
Sort them by Earlier -> Up, Longer -> Left.
And then group them who are being overlapped each other.
And give number by order then stretch them to the same size except the last one. last one could be greedy like he can eat all the rest place!
Performance
200 of random events
original
with new algorithm
1000 of random events
original
with new algorithm
About the Optimization
I leave some comments like TODO where further optimization would be effective.
But as you can see above Performance Metrics, even a thousand of events doesn't affect much.
in case you want to know how A THOUSAND of events looks like,
The source might be kinda dirty and hard to read because I hardly use the
map()for loop which would be prettier. Honestly it causes a performance issue. Here you can find why I usedfor.https://github.com/dg92/Performance-Analysis-JS
And...
I put some more like padding between events. like
2pxbetween them.I think it's prettier but also worry bout it's hard coded.